-
Notifications
You must be signed in to change notification settings - Fork 0
95 build the calculator page #200
base: dev
Are you sure you want to change the base?
Conversation
…vault, share routine rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude review mm:
High: kommer til å feile ved merge med #189 (pagineringen). Dette er en semantisk konflikt. Git viser ingen tekstkonflikt, men det knekker bygget:
- AddElementsList.tsx importerer SEARCH_CATALOG_MORE_QUERY og kaller fetchMore. #189 sletter den spørringen og endrer Apollo-cachen til én side om gangen, så «Load more» i popupen slutter å fungere eller kompilerer ikke.
- Elements-siden og popupen deler searchCatalog i cachen. Med #189 overskriver de hverandres side.
- Anette spør selv om rekkefølgen (valg 1 eller 2 i PR-teksten). Mitt råd: merge #189 først, og la Anette gi popupen en egen enkel løsning, for eksempel en query som ikke deler cache-nøkkel med Elements-siden.
Low: Popupen skriver Showing 20 of {totalCount} uten formatTotal, så tallet kan vise 1001 i stedet for «1000+» (nås ikke med dagens 562 elementer). Å bytte apparat på en tom rutine tilbakestiller navnet, også om brukeren har endret det. Vanskelighetene bruker håndskrevne typer i calculatorOperations.ts i stedet for codegen, men searchOperations.ts gjør det samme, så det er ikke noe avvik.
optional: Hvis dere ønsker en felles dialog senere, ville jeg trukket ut bare skallet (Portal, Backdrop, Popup-posisjonen) og latt de to innholdene være.
…say 1000+ in the popup
|
Thanks! I've fixed the small points ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be merged after 189, lgtm just solve conflicts
Closes #95.
The calculator page builds a routine on one apparatus and shows its D-value, with each element marked as counted or not. Nothing is saved yet: the "Save to a gymnast" button is a placeholder for #152.
The layout follows the Figma design, not the issue text
The issue description describes a different layout than the Figma design. I built it from the Figma design, so the page differs from the issue in these ways:
The colours are the same as on the gymnast pages and the rest of the site.
What is in this PR
API
scoreRoutine(itemIds)returns the D-score, which items count (counted), and which element the dismount replaced (replacedByDismount).calculateDScoregives the same score as before.scoreItemsinbackend/src/dscore.ts: the eight highest values, each code once, and the dismount always counts. If the dismount isn't among the eight highest, it takes the place of the lowest of them.Routine.dScorenow counts the dismount the same way.Calculator page
Changes outside the calculator
ApparatusTabs: new pill look. The gymnast page still uses the line look and is unchanged.ConfirmDialog: long button text now wraps inside the button.TrashButton: only its comment changed.shared/routines.ts: the 50-item limit, the Mounts/Dismounts group names andapparatusOfId, shared by the backend and the frontend.searchOperations.ts: the search query now also asks forremoved, so the popup can turn off removed elements. The Elements page doesn't use it.App.tsx: wrapped inCalculatorRoutineProvider, so the calculator keeps its routine when you go to another page and back. The routes are unchanged.Questions for the reviewers
Pagination (#162). The popup's element list uses "Load more elements", copied from the Elements page's search (marked
TODO(#162)inAddElementsList.tsx). It should use the same pagination as #162. What do you prefer?Not in this PR (follow-ups)
docs/UI_PLANS.md).setRoutineItemsdoesn't check this.How to test
npm run codegeninbackend/(the schema has a new query), then start the backend and the frontend.Checks
--no-file-parallelism: in parallel, two files report rows that other files added to the same database.🤖 Generated with Claude Code